-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BEAM-6207] Added option to publish synthetic data to Kafka topic. #7612
[BEAM-6207] Added option to publish synthetic data to Kafka topic. #7612
Conversation
Hi @kkucharc , could you look at it? Thanks! |
db59671
to
a87a869
Compare
One test failure is in Task :beam-runners-google-cloud-dataflow-java-legacy-worker:test, unrelated to these changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, thanks! I just added two suggestions.
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
Run Java PreCommit |
a87a869
to
b2b0b28
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Posted some suggestions (hopefully helpful). Thanks!
EDIT: forgot one thing: coudl you change a name to something abstracting from PubSub
, like SyntheticDataPublisher
or similar?
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
...ing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPubSubPublisher.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost done, some suggestions and questions emerged though. Thanks!
...a/testing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPublisher.java
Outdated
Show resolved
Hide resolved
...a/testing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPublisher.java
Show resolved
Hide resolved
...a/testing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPublisher.java
Outdated
Show resolved
Hide resolved
...a/testing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPublisher.java
Outdated
Show resolved
Hide resolved
...a/testing/load-tests/src/main/java/org/apache/beam/sdk/loadtests/SyntheticDataPublisher.java
Show resolved
Hide resolved
ffaac57
to
07bc1c6
Compare
SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option which switches it to Kafka publishing mode instead of Google PubSub mode. Key-Value pairs are published as strings.
SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option which switches it to Kafka publishing mode instead of Google PubSub mode. Key-Value pairs are published as strings.
07bc1c6
to
7e1f775
Compare
LGTM, thanks! Merging. |
SyntheticDataPubSubPublisher now accepts --kafkaBootstrapServerAddress option
which switches it to Kafka publishing mode instead of Google PubSub mode.
Key-Value pairs are published as strings.
Follow this checklist to help us incorporate your contribution quickly and easily:
[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.It will help us expedite review of your Pull Request if you tag someone (e.g.
@username
) to look at it.Post-Commit Tests Status (on master branch)